Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix parsing of attribute 188 on seagate drives #527

Merged
merged 1 commit into from
Oct 17, 2023

Conversation

kaysond
Copy link
Contributor

@kaysond kaysond commented Oct 15, 2023

Fixes #522

@codecov-commenter
Copy link

Codecov Report

Merging #527 (550cd59) into master (0febe3f) will decrease coverage by 1.80%.
Report is 66 commits behind head on master.
The diff coverage is 1.81%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##           master     #527      +/-   ##
==========================================
- Coverage   32.54%   30.74%   -1.80%     
==========================================
  Files          54       29      -25     
  Lines        3045     2709     -336     
  Branches       66        0      -66     
==========================================
- Hits          991      833     -158     
+ Misses       2018     1846     -172     
+ Partials       36       30       -6     
Flag Coverage Δ
unittests 30.74% <1.81%> (-1.80%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
webapp/backend/pkg/web/server.go 62.26% <100.00%> (-4.41%) ⬇️
webapp/backend/pkg/config/config.go 0.00% <0.00%> (ø)
webapp/backend/pkg/notify/notify.go 35.39% <0.00%> (-0.37%) ⬇️
...end/pkg/database/scrutiny_repository_migrations.go 0.00% <0.00%> (ø)
webapp/backend/pkg/database/scrutiny_repository.go 11.65% <0.00%> (-0.96%) ⬇️

... and 25 files with indirect coverage changes

@AnalogJ
Copy link
Owner

AnalogJ commented Oct 17, 2023

this looks fantastic, thanks @kaysond

@AnalogJ AnalogJ merged commit ab7fd10 into AnalogJ:master Oct 17, 2023
return rawValue
}
}
if int_pieces[2] >= int_pieces[1] && int_pieces[1] >= int_pieces[0] {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic is slightly flawed. The first number indicates total command timeouts, the second indicates all commands that took over 5 seconds to complete, and the third all commands that took over 7.5 seconds to complete.

See:

Raw Usage

Raw [1 - 0] = Total # of command timeouts, with Max hold of FFFFh

Raw [3 - 2] = Total # of commands with > 5 second completion, including those > 7.5 seconds

Raw [5 - 4] = Total # of commands with > 7.5 second completion

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's actually correct. When the string gets split, the indices are backwards from the byte order, and the int pieces variable uses that same backwards ordering

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kaysond Thank you for the clarification!

@BryanPinsker
Copy link

BryanPinsker commented Oct 17, 2023

In v0.7.2 I am still seeing the same failure on my seagate exos:
image

While, smart continues to report OK.

@firasdib
Copy link

Does this assume that you use something along the lines of this?

 - device: /dev/sde
   type: 'sat'
   commands:
      metrics_smart_args: '-xv 188,raw16 --xall --json -T permissive'

@BryanPinsker
Copy link

This was my error, the issue is resolved for me on the latest beta version.

@kaysond
Copy link
Contributor Author

kaysond commented Oct 17, 2023

Does this assume that you use something along the lines of this?

 - device: /dev/sde
   type: 'sat'
   commands:
      metrics_smart_args: '-xv 188,raw16 --xall --json -T permissive'

@firasdib depends. Smartctl automatically applies those arguments for seagate drives based on a few patterns. If for some reason your drive doesn't match, you'll have to add the arguments. In my case, I didn't need them

@lexxxel
Copy link

lexxxel commented Oct 23, 2023

@firasdib and @kaysond thanks for the post, my ST8000VN004-2M2101 did not fall into the automatic pattern. With the manual change, everything worked as expected.

PS: I see that discussion went on in the bug thread, sorry.

@honmashinsei
Copy link

still here, see #655
I'm having issues with my Seagate Ironwolf Pro 16TB (ST16000NE000-2RW103).
Other Seagate drives are fine (so far)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Seagate Drive Command Timeout with Huge Raw Value
7 participants